-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support opt-in rules #345
support opt-in rules #345
Conversation
Awesome! 👍 |
This is great! So |
self.rules = rules.filter { rule in | ||
let id = rule.dynamicType.description.identifier | ||
if validDisabledRules.contains(id) { return false } | ||
return enabledRules.contains(id) || !(rule is OptInRule) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to have the OptInRule
or possibly a different mechanism for it. It makes enablesRules
only useful for opting in to non-default rules, rather a definitive whitelist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. That's not what I was going for, and not what I outlined in #256 (comment).
What you want is a definitive list of rules to run, ignoring enabled_rules
and disabled_rules
, which now reading back that comment I linked to, was the motivation for a rules
config option.
I see value in that, but it does have to be weighed against the complexity of having a white list, a black list, and a list list 😞 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it, although I got a different meaning from @scottrhoyt 's comment #256 (comment) which seems to imply either a white list or black list, but not both (or opt in).
Closes #256 /cc @keith
We could rules conform to this that are either not universally accepted, or applicable (e.g. #245 & #321) and rules that have too high of a false positive rate (e.g. #201 & #206).